feat: audit fixes — graceful OTEL, async-safe plugins, URL validation, +100 tests#36
Merged
haasonsaas merged 8 commits intomainfrom Mar 11, 2026
Merged
feat: audit fixes — graceful OTEL, async-safe plugins, URL validation, +100 tests#36haasonsaas merged 8 commits intomainfrom
haasonsaas merged 8 commits intomainfrom
Conversation
…, +100 tests - OTEL init no longer panics on misconfigured endpoints; falls back gracefully - Semgrep/ESLint plugins use tokio::spawn_blocking to avoid blocking the runtime - Added semgrep --timeout flag (30s) for process execution safety - Git clone URL validation: only https://, git@, and http://*.git accepted - Added PartialEq derive to ContextType for test assertions - 100+ new tests across storage_json, interactive commands, plugins, CLI commands (pr.rs, git.rs), and duplicate_filter Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dedup severity, empty LLM responses
Bug 1: `is_comment_line` missed bare `#` (shell/Makefile/Python comments)
- Changed `#!` → `#![` in non-comment prefixes (Rust inner attrs always have `[`)
- Any `#` line not matching a known code prefix is now treated as a comment
Bug 2: `find_function_end` ignored single-quoted strings
- `'{'` in JS/Python strings was counted as a real brace, breaking function
boundary detection. Now tracks both `"` and `'` string delimiters.
Bug 3: `deduplicate_comments` could drop higher-severity comments
- `dedup_by` keeps the first element; sort didn't include severity.
Now sorts by severity (highest first) within same file/line/content group.
Bug 4: OpenAI/Anthropic silently returned empty string for empty responses
- Empty `choices`/`content` arrays now return errors instead of succeeding
with empty content, preventing silent failures in the review pipeline.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… file status - Error rate now counts only explicit review.failed events, not all non-completed - Empty lines in diffs treated as context instead of being silently skipped - parse_text_diff sets is_new/is_deleted based on empty old/new content - Default model changed from Sonnet to Opus per project conventions - Added 5 new tests covering all discovered issues Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Skip single-quote tracking for Rust in find_function_end (lifetimes like 'a/'static are not string delimiters, were breaking brace count) - Add #!/ to HASH_NON_COMMENT_PREFIXES so shebang lines are not misclassified as comments (changing interpreter is a real code change) - Added 2 regression tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lean up comments - Fix DeduplicateStage in composable_pipeline.rs to preserve highest severity when deduplicating (same bug as comment.rs, different path) - Accept ssh:// URLs in is_safe_git_url (was rejecting legitimate SSH) - Consolidate is_git_source to delegate to is_safe_git_url - Replace stale "BUG:" test comments with "Regression:" phrasing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Wrap semgrep/eslint spawn_blocking in tokio::time::timeout to prevent hanging subprocesses from permanently consuming blocking pool threads - Replace false-positive semgrep test (tested struct construction, not analyzer) with actual timeout behavior test - Replace eslint extension test with direct JS_EXTENSIONS unit test that verifies filter membership rather than relying on end-to-end run - Fix DeduplicateStage in composable_pipeline to preserve highest severity (same bug as comment.rs dedup, different code path) - Accept ssh:// URLs in is_safe_git_url - Clean up stale BUG: comments to Regression: phrasing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Both backends now count only explicit review.failed events as failures, not all non-completed events. Fixes inconsistency where PG counted timeouts as failures but JSON did not. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🔴 CLI --model default still uses claude-sonnet-4-6, overriding the config default change to Opus
The default_model() in src/config.rs:1064 was changed to "claude-opus-4-6" to comply with CLAUDE.md ("Use frontier models for reviews — never default to smaller models"). However, the CLI argument at src/main.rs:33 still has default_value = "claude-sonnet-4-6". Since main.rs:356 always calls config.merge_with_cli(Some(cli.model.clone()), ...) — and clap always provides a value when default_value is set — the CLI default of Sonnet will always override the config-level Opus default. This makes the default_model() change in this PR effectively dead code for all CLI users.
How the override chain works
Config::load()deserializes withdefault_model()→"claude-opus-4-6"merge_with_cli(Some("claude-sonnet-4-6"), ...)overwritesself.modelunconditionally- User gets Sonnet, not Opus, unless they explicitly pass
--model
(Refers to line 33)
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
init_otel()no longer panics on misconfigured endpoints — logs a warning and continues without tracingtokio::task::spawn_blockingto avoid blocking the async runtime; semgrep gets a 30s--timeoutflagprepare_pattern_repository_checkout()now rejects non-https/git@ URLs viais_safe_git_url(), preventing command injection through malicious repo sourcesTest plan
cargo test— 970 tests passingcargo clippy— cleancargo fmt --check— clean🤖 Generated with Claude Code